-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
implementation/tests of the template inheritance mechanism #336
base: main
Are you sure you want to change the base?
Conversation
… what offers Django
cc @Shopify/liquid |
lib/liquid/tags/inherited_block.rb
Outdated
|
||
end | ||
|
||
class InheritedBlockDrop < Drop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you put this into it's own file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree but where? Any thoughts?
@tylerball, can you check it out and see if this is close to what you had in mind? |
hi @fw42, I pushed a new version of my PR. It reflects the modifications you brought up in your comments. Let me know if you have other comments / questions. |
lib/liquid/tags/extends.rb
Outdated
# {% block content }Hello world{% endblock %} | ||
# | ||
class Extends < Block | ||
Syntax = /(#{QuotedFragment}+)/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs memoization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @tobi! do you mean for the overall implementation or in some specific methods?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the regexp. It's done by adding /o to it.
http://stackoverflow.com/questions/13334807/what-does-the-o-modifier-for-a-regexp-mean
- tobi
CEO Shopify
On Wed, Apr 9, 2014 at 10:00 AM, Didier Lafforgue
notifications@github.comwrote:
In lib/liquid/tags/extends.rb:
@@ -0,0 +1,76 @@
+module Liquid
+
Extends allows designer to use template inheritance
{% extends home %}
{% block content }Hello world{% endblock %}
- class Extends < Block
- Syntax = /(#{QuotedFragment}+)/
hi @tobi https://github.com/tobi! do you mean for the overall
implementation or in some specific methods?—
Reply to this email directly or view it on GitHubhttps://github.com//pull/336/files#r11436430
.
This is a large departure from the current way to use liquid. It's implemented well but it will divide liquid users into two camps: The more traditional composition and the Django style block approach. Should this perhaps be an addon gem? |
I understand your concern about this feature, knowing that this might have "consequences" for your Shopify customers. |
@tobi, is that possible this add-on becomes an "official" gem in a sense its sources would be stored in your github organization so that people can contribute / maintain it in more official way than it's actually. Because I've got this code for a couple of years in my forked version of Liquid but it was kind of hidden finally. |
# {% block content }Hello world{% endblock %} | ||
# | ||
class InheritedBlock < Block | ||
Syntax = /(#{QuotedFragment}+)/o |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be SYNTAX
, why do we use that style everywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea :-) I just followed the pattern of the other existing tags.
btw @fw42 & @tylerball, it works now with liquid 3.0. |
hey @jeroenvisser101, I think they changed a couple of stuff in their API. |
@did thanks! I'll have a look at that for sure. My best outcome would be to get it working with the 'original' version of liquid, and to get it working with the master branch (there's some things that changed there, options doesn't seem to be writable anymore, but I'll have to investigate more) I'll let you know when I've got something solid. I don't think it'll be able to be merged into locomotivecms's fork, since it might not work with the liquid fork, but let's have a look when/if I get this to work. |
Do we have a plan to merge it at some point? Since it's coming to the end of 2021 and roughly 7 years passed, and this feature really helps to facilitate the works to maintain a bunch of templates that people don't need to copy and paste again and again those same snippets everywhere :) |
hi @fw42 & @tylerball,
Based on the conversation here (#264), this is my implementation of template inheritance, similar to what offers Django (https://docs.djangoproject.com/en/dev/topics/templates/).
Code mainly extracted from the LocomotiveCMS fork of Liquid.
Feel free to comment and ask me questions.
Thanks!